Move disclosures to fetch-logic #8008
Conversation
wpears
force-pushed
the
fetch-disclosures
branch
from
3e937fe to
715199f
Compare
| @@ -83,7 +83,7 @@ <h2 class="verify_prompt"> | |||
| </h2> | |||
| <form class="verify_form"> | |||
| <div class="verify_school"> | |||
| {{school.primary_alias}} | |||
| {% if school%}{{school.primary_alias}}{% endif %} | |||
There was a problem hiding this comment.
Instead of all the conditionals, would it make sense to have something like this at the top:
{% set school_primary_alias = school.primary_alias if school else '' %}
And then {{ school_primary_alias }} in the spots where it is used?
If you do keep the conditions, perhaps format them the same as {% if school %}, instead of a mix of that and {% if school%}
There was a problem hiding this comment.
Sure, I just did it with a mass replace in vim ^^;
| @@ -130,16 +89,14 @@ const getApiValues = { | |||
| const warning = document | |||
| .querySelector('[data-warning]') | |||
| .getAttribute('data-warning'); | |||
| if (warning !== '' && typeof warning !== 'undefined') { | |||
| if (warning && !window.Cypress) { | |||
There was a problem hiding this comment.
Should this code know about Cypress? It seems like whatever the need is here should be offloaded to the test logic.
There was a problem hiding this comment.
Yeah this is kinda gross, the problem is though that the data isn't available in the test db, so this warning code will always trigger, even though the actual http requests are stubbed.
Putting this logic here allows Cypress to skip the warning code (which we want) and, in normal use of the app, allows the warning conditional to trigger when we need it.
I think maybe the best solution would be rearchitecting how the warning logic works/how the warning is propagated from the backend to the frontend, but that seemed out of scope.
There was a problem hiding this comment.
Yeah, I'd be happy to fast-follow that because this is pretty gross. 😁
@anselmbradford Suggested we could switch to fetch here and I agree!
cypress tests should pass and the app should work identically to the disclosures branch
Also fixes a few random bugs that I found, eg, guards
schoolaccess in the template,elementsaccess in dollar-sign.js, and fixesfinancialInputChangeListenerto target a real selector.